Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(platform): re-attempt platform automerge on github and gitlab #26567

Merged
merged 26 commits into from
Mar 18, 2024
Merged

feat(platform): re-attempt platform automerge on github and gitlab #26567

merged 26 commits into from
Mar 18, 2024

Conversation

straub
Copy link
Contributor

@straub straub commented Jan 9, 2024 •
edited
Loading

Changes

This change will cause Renovate to re-enable GitHub's automerge / Merge Queue feature after rebasing the PR, to more seamlessly recover when PRs encounter merge conflicts in the queue and are removed by github-merge-queue[bot].

This is a naive first pass at the feature, and I'm submitting this PR as a work-in-progress to get feedback. I suspect, among other potential feedback, that my testing needs to be improved and that it's possible the new feature should be opt-in via a new config flag (not yet added).

Thanks for your time!

Context

#26523

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@HonkingGoose
Copy link
Collaborator

I'm unsure whether this is needed. I have not included a doc update in this PR yet.

The new behavior should be explained in Renovate's platformAutomerge docs. Currently those docs say:

Renovate tries platform-native automerge only when it initially creates the PR. Any PR that is being updated will be automerged with the Renovate-based automerge.

But your PR makes it so that Renovate enables the platform native automerge (GitHub Merge queue in this case) again after a merge conflict, I think. So that should be explained.

rarkins
rarkins reviewed Jan 10, 2024
View reviewed changes
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the refreshPr for bitbucket is such a different use - and called at different times where re-enabling automerge is not relevant - that it's probably not a good idea to combine it

@straub
Copy link
Contributor Author

straub commented Jan 10, 2024

I've refactored this implementation to avoid using/changing refreshPr. Now using a new method called reattemptPlatformAutomerge()

@straub straub marked this pull request as ready for review January 10, 2024 15:23
HonkingGoose
HonkingGoose suggested changes Jan 10, 2024
View reviewed changes
Copy link
Collaborator

@HonkingGoose HonkingGoose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs more work after applying the suggestion.

rarkins
rarkins reviewed Feb 18, 2024
View reviewed changes
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but needs tests passing

@straub
Copy link
Contributor Author

straub commented Feb 29, 2024 •
edited
Loading

LGTM but needs tests passing

Tests are now passing! Please let me know if this needs anything else

@straub straub requested a review from rarkins February 29, 2024 17:00
rarkins
rarkins reviewed Mar 1, 2024
View reviewed changes
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there still any concern about feature parity with gitlab? I recall a comment being added her earlier about it

@straub
Copy link
Contributor Author

straub commented Mar 1, 2024 •
edited
Loading

Is there still any concern about feature parity with gitlab? I recall a comment being added her earlier about it

I was looking for some direction on that in relation to the docs:

  1. feat(platform): re-attempt platform automerge on github and gitlab  #26567 (comment)
  2. feat(platform): re-attempt platform automerge on github and gitlab  #26567 (comment)

It's not strictly a part of this change, but I'd be happy to cover it while I'm in here

viceice
viceice previously approved these changes Mar 16, 2024
View reviewed changes
@viceice viceice changed the title feat(platform/github): re-attempt platform automerge feat(platform): re-attempt platform automerge on github and gitlab Mar 18, 2024
@rarkins rarkins added this pull request to the merge queue Mar 18, 2024
Merged via the queue into renovatebot:main with commit ebf0c7b Mar 18, 2024
37 checks passed
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 37.255.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@straub straub deleted the feat/github-refreshPr-tryPrAutomerge branch March 22, 2024 17:01
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@zharinov zharinov zharinov left review comments

@viceice viceice viceice approved these changes

@HonkingGoose HonkingGoose HonkingGoose approved these changes

@rarkins rarkins Awaiting requested review from rarkins

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@straub @HonkingGoose @renovate-release @zharinov @viceice @rarkins